Import GRUB static migration code #790

travier · 2024-12-03T19:04:17Z

Fixes: #789

openshift-ci · 2024-12-03T19:04:30Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

cgwalters

Thanks for this!

cgwalters · 2024-12-03T19:06:39Z

src/bootupd.rs

@@ -489,6 +496,88 @@ pub(crate) fn client_run_validate() -> Result<()> {
    Ok(())
 }

+pub(crate) fn client_run_migrate() -> Result<()> {
+    // Used to condition execution of this unit at the systemd level
+    let stamp_file="/var/lib/.fedora_atomic_desktops_static_grub";


Missing a cargo fmt.

Also... now that this is part of bootupd calling the stamp file name "fedora_atomic_desktops_" seems odd.

Bikeshedding things a bit more...bootupd already has its own little database where we could store this state.

I'm also fine just keeping it as a stamp file, but how about e.g. .bootupd-static-migration-complete? Also since this is about data in /boot I think we should probably keep the stamp file there?

If we want to have this represented in bootup state directly then we would have to add a new mode to distinguish between the bootupd managed static grub configs and the ones that we imported from a system that are still user managed because they may contain arbitrary changes, os-prober systems, etc. and we don't want bootupd to override those on static config updates (when we'll implement that).

Hmm...I more meant that we add a new key to the JSON file

So in the end, I have not done that yet as:

I would have to figure out how to do it and test it

Having a stamp file makes it easy to skip running this later during boot using systemd

src/bootupd.rs

travier · 2024-12-04T15:41:01Z

I immediately ran into fedora-selinux/selinux-policy#2444 while testing this.

HuijingHei · 2024-12-06T07:53:27Z

Overall LGTM, I can help to do some testing if you have some instructions.

travier · 2024-12-12T18:16:44Z

How I'm (manually) testing this change:

Install Fedora Silverblue 40 in two VMs, one using BIOS & another UEFI (see https://gitlab.com/fedora/ostree/scripts for scripts that automate that), do the same for Fedora Silverblue 41
Update the Fedora 40 VMs to Fedora 41
Optional: Make a snapshot of the VM
Build bootupd and copy it to the VM:

$ cargo build --release
$ scp target/release/bootupd silverblue-41-uefi-bootupd:
[silverblue]$ sudo rpm-ostree usroverlay
[silverblue]$ sudo cp bootupd /usr/bin/bootupctl && sudo cp bootupd /usr/libexec/bootupd

Run the migration:

$ sudo bootupctl migrate

Validate the state of the system:
- Things to verify, on a system installed from Fedora 41:

root@fedora:~# sudo ls -alh /boot/grub2
total 16K
drwx------. 2 root root 4.0K Oct 25 13:29 .
drwxr-xr-x. 7 root root 4.0K Nov 29 14:37 ..
-rw-r--r--. 1 root root   53 Oct 25 13:29 bootuuid.cfg
-rw-r--r--. 1 root root 2.6K Oct 25 13:29 grub.cfg

root@fedora:~# grep ostree /boot/grub2/grub.cfg
<empty>

root@fedora:~# grep blscfg /boot/grub2/grub.cfg
blscfg

root@fedora:~# sudo cat /sysroot/ostree/repo/config
[core]
repo_version=1
mode=bare

[sysroot]
readonly=true
bootloader=none

Things to verify, on a system installed from an older release:

root@fedora:~# ls -alh /boot/grub2
total 64K
drwx------. 5 root root 4.0K Dec  2 20:30 .
drwxr-xr-x. 6 root root 4.0K Dec  2 20:13 ..
-rw-r--r--. 1 root root   64 Oct 18 17:50 device.map
drwx------. 2 root root 4.0K Jan  1  1970 fonts
-rw-r--r--. 1 root root    0 Dec  2 20:30 .grub2-blscfg-supported
-rw-------. 1 root root 7.0K Dec  2 20:30 grub.cfg
-rw-------. 1 root root 8.8K Nov 29 10:15 grub.cfg.backup
-rw-------. 1 root root 1.0K Dec  2 15:22 grubenv
drwxr-xr-x. 2 root root  20K Dec  2 20:13 i386-pc
drwxr-xr-x. 2 root root 4.0K Dec  2 20:13 locale

root@fedora:~# ls -alh /boot/grub2/.grub2-blscfg-supported 
-rw-r--r--. 1 root root 0 Dec  2 20:30 /boot/grub2/.grub2-blscfg-supported

root@fedora:~# grep ostree /boot/grub2/grub.cfg
  set kernelopts="root=UUID=f59453a5-036d-4b9a-b1a2-4428bf5eaecf ro rootflags=subvol=root/ostree/deploy/fedora/deploy/a7eefc0751cf4688e6daf0d998d24a435ae4d44ab193d8dce7e01f66d366fa08.0 rhgb quiet "
### BEGIN /etc/grub.d/15_ostree ###
### END /etc/grub.d/15_ostree ###

root@fedora:~# grep blscfg /boot/grub2/grub.cfg
# The blscfg command parses the BootLoaderSpec files stored in /boot/loader/entries and
insmod blscfg
blscfg

root@fedora:~# cat /sysroot/ostree/repo/config
[core]
repo_version=1
mode=bare

[sysroot]
readonly=true
bootloader=none

travier · 2024-12-12T18:39:00Z

So this works as far as I've tested but we have no tests for it yet.

HuijingHei · 2024-12-13T14:30:10Z

Install Fedora Silverblue 40 in two VMs, one using BIOS & another UEFI (see https://gitlab.com/fedora/ostree/scripts for scripts that automate that)
Update the Fedora 40 VMs to Fedora 41

Do testing on BIOS VM and UEFI VM, then upgrade to f41, copy the new bootupd to the vm, run $ sudo bootupctl migrate, check the results are expected as above, also remove ostree-grub2 (seems it is removed in https://pagure.io/workstation-ostree-config/pull-request/591), and reboot successfully.
Let me know if I missed something, thanks!

travier · 2024-12-18T11:43:27Z

I've added the systemd unit to this PR and rebased it on top of #803 to avoid conflicts for the systemd unit setup in the Makefile & specfile.

I've split the clippy lint fixes in #804.

cgwalters · 2024-12-18T13:23:10Z

contrib/packaging/bootupd-static-grub-migration.service

+Documentation=https://github.com/coreos/bootupd
+ConditionPathExists=!/boot/.bootupd-static-migration-complete
+RequiresMountsFor=/sysroot /boot
+# Only run after a successful bootloader update


Are there any systems which would enable the static migration but not the update service?

If not, why wouldn't we just fold this functionality into the bootloader-update.service via

ExecStart=bootupctl migrate ExecStart=bootupctl update

Or I guess just have bootupctl update --migrate-if-needed or so.

We indeed want all the systems that do the migration to update their bootloader before.

We don't need to do the migration on systems which already have a static GRUB configs and we can check the state for that, so maybe this should indeed be folded into a single command and moved from a stamp file to a field in the state JSON.

If we merge both units then it also means that we have to careful when rolling this out as we won't be easily able to enable one or the other independently. But that's also kind of OK as I'm looking at doing that in F41 as well before the F42 release.

src/bootupd.rs

See: https://gitlab.com/fedora/ostree/sig/-/issues/35 See: https://pagure.io/workstation-ostree-config/pull-request/591 Fixes: coreos#789

Note that the service is intentionally not enabled by default as it should be up to the distribution to add a systemd preset if the migration to a static GRUB config is needed. This will be used on Atomic Desktops & IoT systems to migrate systems to a static GRUB config before enabling composefs as GRUB curently does not interact well with it: https://bugzilla.redhat.com/show_bug.cgi?id=2308594 See: coreos#789 See: https://fedoraproject.org/wiki/Changes/ComposefsAtomicDesktops

HuijingHei · 2024-12-19T09:06:10Z

Ask a silly question, is there any reason should do the migration only when /boot/grub2/grub.cfg is symlink?

Check on silverblue40 (using efi), it is symlink to /boot/loader/grub.cfg, but on silverblue41, it is a normal file.

travier · 2024-12-19T14:09:09Z

Ask a silly question, is there any reason should do the migration only when /boot/grub2/grub.cfg is symlink?

As far I know, this is the marker that lets us tell a system with a dynamic config from one with a static one.

Check on silverblue40 (using efi), it is symlink to /boot/loader/grub.cfg, but on silverblue41, it is a normal file.

On Silverblue 40, GRUB is setup using a dynamic config like package mode Fedora and on Silverblue 41, it is setup by bootupd with a static config.

openshift-ci bot added the do-not-merge/work-in-progress label Dec 3, 2024

cgwalters reviewed Dec 3, 2024

View reviewed changes

travier mentioned this pull request Dec 4, 2024

Make bootupd_t permissive fedora-selinux/selinux-policy#2444

Merged

travier force-pushed the main-static-migration branch 6 times, most recently from 0436363 to ed15ca7 Compare December 5, 2024 17:07

travier force-pushed the main-static-migration branch 2 times, most recently from ec3c25f to e17023d Compare December 12, 2024 18:26

travier marked this pull request as ready for review December 12, 2024 18:34

travier force-pushed the main-static-migration branch from e17023d to 4a65f3b Compare December 18, 2024 11:42

travier changed the title ~~WIP: Import GRUB static migration code~~ Import GRUB static migration code Dec 18, 2024

openshift-ci bot removed the do-not-merge/work-in-progress label Dec 18, 2024

cgwalters reviewed Dec 18, 2024

View reviewed changes

travier added 2 commits December 18, 2024 16:37

migrate: Add GRUB static migration subcommand

3926dcf

See: https://gitlab.com/fedora/ostree/sig/-/issues/35 See: https://pagure.io/workstation-ostree-config/pull-request/591 Fixes: coreos#789

travier force-pushed the main-static-migration branch from 4a65f3b to 06132d0 Compare December 18, 2024 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import GRUB static migration code #790

Import GRUB static migration code #790

travier commented Dec 3, 2024 •

edited

Loading

openshift-ci bot commented Dec 3, 2024

cgwalters left a comment

cgwalters Dec 3, 2024

travier Dec 4, 2024 •

edited

Loading

cgwalters Dec 4, 2024

travier Dec 12, 2024

travier commented Dec 4, 2024

HuijingHei commented Dec 6, 2024

travier commented Dec 12, 2024

travier commented Dec 12, 2024

HuijingHei commented Dec 13, 2024

travier commented Dec 18, 2024

cgwalters Dec 18, 2024

travier Dec 18, 2024

HuijingHei commented Dec 19, 2024

travier commented Dec 19, 2024

Import GRUB static migration code #790

Are you sure you want to change the base?

Import GRUB static migration code #790

Conversation

travier commented Dec 3, 2024 • edited Loading

openshift-ci bot commented Dec 3, 2024

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Dec 3, 2024

Choose a reason for hiding this comment

travier Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

cgwalters Dec 4, 2024

Choose a reason for hiding this comment

travier Dec 12, 2024

Choose a reason for hiding this comment

travier commented Dec 4, 2024

HuijingHei commented Dec 6, 2024

travier commented Dec 12, 2024

travier commented Dec 12, 2024

HuijingHei commented Dec 13, 2024

travier commented Dec 18, 2024

cgwalters Dec 18, 2024

Choose a reason for hiding this comment

travier Dec 18, 2024

Choose a reason for hiding this comment

HuijingHei commented Dec 19, 2024

travier commented Dec 19, 2024

travier commented Dec 3, 2024 •

edited

Loading

travier Dec 4, 2024 •

edited

Loading